Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[GCU] Update RDMA Platform Validator #2913

Merged
merged 12 commits into from
Jul 20, 2023
Merged

[GCU] Update RDMA Platform Validator #2913

merged 12 commits into from
Jul 20, 2023

Conversation

isabelmsft
Copy link
Contributor

@isabelmsft isabelmsft commented Jul 14, 2023 •
edited
Loading

What I did

ADO 24509483
-Fix RDMA platform validator PR to consider two edge cases:

  1. Allow empty string value "" when the operation is remove in jsonpatch. Previously, empty string was not an allowed value, so these remove operations were incorrectly blocked. This is fixed by adding the empty string "" in the platform validator conf file for RDMA tables where remove operation is supported (only PFC_WD).
  2. Ensure proper string parsing for an edge case jsonpatch path and value combination. The edge case combination is: the path includes contains a variable digit (such as /PFC_WD/Ethernet64 where 64 is the variable digit) and value is a dict. In this scenario, we need to clean the path and value combination differently so that the relevant value is properly extracted without an extra "/".

-add support for Mellanox spc2, spc3 asic

How I did it

Update GCU x RDMA platform validator functionality

How to verify it

E2E testing, test_add_rack passes

============================= test session starts ==============================
platform linux2 -- Python 2.7.18, pytest-4.6.11, py-1.11.0, pluggy-0.13.1
ansible: 2.8.12
rootdir: /data/sonic-mgmt-int/tests, inifile: pytest.ini
plugins: metadata-1.11.0, allure-pytest-2.8.22, html-1.22.1, xdist-1.28.0, forked-1.3.0, celery-4.4.7, repeat-0.9.1, ansible-2.2.4
['conf-name', 'group-name', 'topo', 'ptf_image_name', 'ptf', 'ptf_ip', 'ptf_ipv6', 'server', 'vm_base', 'dut', 'inv_name', 'auto_recover', 'comment']
Finished testbed info generating.
collected 1 item

configlet/test_add_rack.py::test_add_rack PASSED                         [100%]

test_pfcwd_status

============================= test session starts ==============================
platform linux2 -- Python 2.7.18, pytest-4.6.11, py-1.11.0, pluggy-0.13.1
ansible: 2.8.12
rootdir: /data/sonic-mgmt-int/tests, inifile: pytest.ini
plugins: metadata-1.11.0, allure-pytest-2.8.22, html-1.22.1, xdist-1.28.0, forked-1.3.0, celery-4.4.7, repeat-0.9.1, ansible-2.2.4
['conf-name', 'group-name', 'topo', 'ptf_image_name', 'ptf', 'ptf_ip', 'ptf_ipv6', 'server', 'vm_base', 'dut', 'inv_name', 'auto_recover', 'comment']
Finished testbed info generating.
collected 4 items

generic_config_updater/test_pfcwd_status.py::test_stop_pfcwd[single] PASSED [ 25%]
generic_config_updater/test_pfcwd_status.py::test_stop_pfcwd[all] PASSED [ 50%]
generic_config_updater/test_pfcwd_status.py::test_start_pfcwd[single] PASSED [ 75%]
generic_config_updater/test_pfcwd_status.py::test_start_pfcwd[all] PASSED [100%]

---------- generated xml file: /data/sonic-mgmt-int/tests/logs/tr.xml ----------

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@isabelmsft isabelmsft changed the title Fix platform validator [GCU] Update RDMA Platform Validator Jul 14, 2023
qiluo-msft
qiluo-msft reviewed Jul 18, 2023
View reviewed changes
generic_config_updater/field_operation_validators.py Outdated
@@ -75,7 +79,7 @@ def _get_fields_in_patch():

if 'value' in patch_element.keys() and isinstance(patch_element['value'], dict):
for key in patch_element['value']:
cleaned_fields.append(cleaned_field+ '/' + key)
cleaned_fields.append(cleaned_field + '/' + key) if len(cleaned_field) > 0 else cleaned_fields.append(key)
Copy link
Contributor

@qiluo-msft qiluo-msft Jul 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if

It's better to use if-else block. Single line is too long. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to if-else block

qiluo-msft
qiluo-msft previously approved these changes Jul 19, 2023
View reviewed changes
wen587
wen587 previously approved these changes Jul 19, 2023
View reviewed changes
@jarias-lfx
Copy link

/easycla

@isabelmsft isabelmsft dismissed stale reviews from wen587 and qiluo-msft via ef0e685 July 20, 2023 00:00
@isabelmsft isabelmsft merged commit e49fd91 into sonic-net:master Jul 20, 2023
rajkumar38 pushed a commit to rajkumar38/sonic-utilities that referenced this pull request Jul 25, 2023
@isabelmsft @rajkumar38
[GCU] Update RDMA Platform Validator (sonic-net#2913)
990d712
isabelmsft added a commit to isabelmsft/sonic-utilities that referenced this pull request Aug 10, 2023
@isabelmsft
[GCU] Update RDMA Platform Validator (sonic-net#2913)
c9c0989
@isabelmsft isabelmsft mentioned this pull request Aug 10, 2023
Draft
@StormLiangMS
Copy link
Contributor

@isabelmsft ADO?

pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
@isabelmsft @pdhruv-marvell
[GCU] Update RDMA Platform Validator (sonic-net#2913)
cb9419d
StormLiangMS pushed a commit that referenced this pull request Sep 3, 2023
@isabelmsft @StormLiangMS
[GCU] Update RDMA Platform Validator (#2913)
c7ff527
isabelmsft added a commit to isabelmsft/sonic-utilities that referenced this pull request Nov 22, 2023
@isabelmsft
[GCU] Update RDMA Platform Validator (sonic-net#2913)
f11df49
@isabelmsft isabelmsft mentioned this pull request Nov 22, 2023
Merged
yxieca pushed a commit that referenced this pull request Nov 29, 2023
@isabelmsft @stephenxs
[GCU Bug Fix] Cherry-pick RDMA Platform Validator PR to 202205 (#3051)
8765fce 
Cherry-pick PR #2692 (set up test infrastructure used in later PR 2857), #2857 (platform validator PR), #2913 (edge case fix), #2951, #3018 (add support for new Mellanox HwSKU in conf file)
This change stops GCU from modifying a protected RDMA field.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
Co-authored-by: Stephen Sun <5379172+stephenxs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neethajohn neethajohn neethajohn approved these changes

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@wen587 wen587 wen587 left review comments

Assignees
No one assigned
Labels
Included in 202205 Branch Included in 202305 Branch Request for 202205 Branch Request for 202305 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@isabelmsft @jarias-lfx @StormLiangMS @wen587 @qiluo-msft @neethajohn @yxieca